Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add into_inner() for HTTP client and server #412

Closed
wants to merge 1 commit into from
Closed

Add into_inner() for HTTP client and server #412

wants to merge 1 commit into from

Conversation

sargun
Copy link
Contributor

@sargun sargun commented Oct 5, 2024

This adds the into_inner() function to the stream type, allowing the HTTP/1.1 client and server to be turned back into a stream.

These functions are useful for implementing HTTP CONNECT or HTTP Upgrade.

@sargun
Copy link
Contributor Author

sargun commented Oct 5, 2024

This addresses #230

@sargun
Copy link
Contributor Author

sargun commented Oct 5, 2024

I do not think the failure is related.

@sargun
Copy link
Contributor Author

sargun commented Oct 5, 2024

I feel like this is a common-ish rust pattern, where you have into_inner(), that then returns the original thing the object was built from. We'd probably want to flush, but I think a reasonable way to make the API is something like:

At least for the HTTP v1 client:

pub async fn into_inner(self) -> Result<Stream> {
   self.underlying_stream
            .flush()
            .await
            .or_err(WriteError, "flushing body")?;
Ok(self.underlying_stream)
}

The other option, which extends our current story is:

fn stream_mut(&mut self) -> &mut Stream {
  &mut self.underlying_stream
}

Curious, what folks find more palatable?

@sargun
Add into_inner() for HTTP client and server
95aa770 
This adds the into_inner() function to the stream type, allowing
the HTTP/1.1 client and server to be turned back into a stream.

These functions are useful for implementing HTTP CONNECT or
HTTP Upgrade.
@drcaramelsyrup drcaramelsyrup added the enhancement New feature or request label Oct 11, 2024
@gumpt gumpt added the Accepted This change is accepted by us and merged to our internal repo label Oct 11, 2024
@sargun
Copy link
Contributor Author

sargun commented Oct 17, 2024

Bump?

drcaramelsyrup pushed a commit that referenced this pull request Oct 18, 2024
@sargun @gumpt
Add into_inner() for HTTP client and server
791183d 
This adds the into_inner() function to the stream type, allowing
the HTTP/1.1 client and server to be turned back into a stream.

These functions are useful for implementing HTTP CONNECT or
HTTP Upgrade.

Includes-commit: 95aa770
Replicated-from: #412
drcaramelsyrup pushed a commit that referenced this pull request Oct 18, 2024
@sargun @drcaramelsyrup
Add into_inner() for HTTP client and server
63af8e2 
This adds the into_inner() function to the stream type, allowing
the HTTP/1.1 client and server to be turned back into a stream.

These functions are useful for implementing HTTP CONNECT or
HTTP Upgrade.

Includes-commit: 95aa770
Replicated-from: #412
@drcaramelsyrup
Copy link
Contributor

Thank you for your contribution! this was merged in 63af8e2.

@sargun sargun deleted the add-http-into-inner branch October 21, 2024 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@gumpt gumpt

Labels
Accepted This change is accepted by us and merged to our internal repo enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sargun @drcaramelsyrup @gumpt